Fix nesting too deep - #401
Conversation
53cb5b4 to
bf0439d
Compare
bf0439d to
012c11d
Compare
|
lmao debug builds are using 80kb of stack per frame? I think we need to fix that. A recursion depth limit of 16 is far too low. I'd prefer no limit at all. Is the recursion inherent to chumsky or can we fix it within our own code. |
|
Yeah, this is a concerning bug though, as wallets will potentially be compiling code. I agree that 16 is too low. Let me dig in and see if I can find out what is taking up so much space. |
|
The compiler shouldn't be recursing here. In We should investigate where the actual stack overflow comes from. |
e2530af parse: parse block bodies in linear time (stringhandler) 73751e4 tests: measure how parse time scales with nesting depth (stringhandler) Pull request description: In an attempt to increase the nesting limit in #401, an LLM found this incredibly subtle bug in statements parsing. It's difficult to explain why this works, basically it would match a block body, then fail and parse each statement again. I've left the extraneous descriptions in the commits, but happy to remove them. There are two commits. Commit 1 adds an ignored test that shows the problem. Run it with `cargo test --test parser_scaling -- --ignored --nocapture` Commit 2 adds the fix that converts it from exponential to linear. Run the test again afterwards to see the improvement. Here are some figures from my computer: ``` running 1 test nesting construct, time per depth, and growth across two levels blocks depth 8 4.85ms - depth 10 16.86ms 3.48x depth 12 62.02ms 3.68x depth 14 243.23ms 3.92x depth 16 989.35ms 4.07x .... ... depth 16 against depth 8 (linear is about 2x, doubling per level is about 256x) blocks 204.1x TOO STEEP parentheses 1.3x ok option types 1.6x ok tuple types 1.4x ok array types 1.4x ok ``` After ``` running 1 test nesting construct, time per depth, and growth across two levels blocks depth 8 1.24ms - depth 10 1.33ms 1.07x depth 12 1.26ms 0.95x depth 14 1.32ms 1.05x depth 16 1.30ms 0.98x ``` ACKs for top commit: apoelstra: ACK e2530af; successfully ran local tests Tree-SHA512: 7b37aace675dcb5d2cbb2430c92f65041d43222b56c58eb4bb6e2662e1e421714ec149330d9a183fdca6abfd6d43d32f49f396f6710d7ef4c979d515fb5acb2f
|
Closing this for now as #410 solves most of the problem, and this wouldn't really be needed |
Fixes #399.
Deeply nested input overflowed the stack and aborted the process instead of producing an error, making any service that compiles .simf files trivially killable.